Fix/removed files persist take 2#12206
Conversation
|
Hi there! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
This adjusts the build script to ensure that any files deleted from the upstream built asset are removed as expected. This also solves a long-standing issue where unversioned files persist in `src` after being eliminated, which can cause `grunt verify:old-files` to fail.
desrosj
force-pushed
the
fix/removed-files-persist-take-2
branch
from
ea9fbd2 to
f4a738b
Compare
desrosj
requested review from
aaronjorbin,
adamsilverstein,
Copilot and
westonruter
There was a problem hiding this comment.
Pull request overview
This PR updates the Gutenberg integration build pipeline so Gutenberg-sourced artifacts are generated into src/ and proactively cleaned, preventing stale/removed upstream files from lingering in src/wp-includes/ across different build flows.
Changes:
- Always copy Gutenberg build outputs into
src/(notWORKING_DIR) to avoidbuildvsbuild:devdrift and stale files. - Run
clean:gutenbergfirst inbuild:gutenberg, expanding it to clean both tracked and ignored Gutenberg-managed files (with safeguards when running plaingrunt clean). - Refactor Gutenberg block copying in
tools/gutenberg/copy.jsinto explicit steps (JSON, PHP, CSS) and update related task grouping inGruntfile.js.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/gutenberg/copy.js | Standardizes Gutenberg copy destination to src/ and splits block copying into JSON/PHP/CSS steps. |
| Gruntfile.js | Reworks Gutenberg-related clean/copy task wiring to clean first and consistently build into src/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Handles both the top-level `<block>.php` dynamic block files and any nested | ||
| * `*.php` helpers under `<block>/` (e.g. `navigation-link/shared/render-submenu-icon.php`). | ||
| * | ||
| * @param {Object} config - Block configuration from `COPY_CONFIG.blocks`. |
There was a problem hiding this comment.
Can a @typedef be added for the shape of this to be reused here and in copyBlockStyles()?
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| const isExplicitGutenbergClean = | ||
| grunt.cli.tasks.length === 1 && | ||
| grunt.cli.tasks[ 0 ] !== 'clean'; | ||
| return isExplicitGutenbergClean ? | ||
| gutenbergUnversionedFiles.concat( gutenbergVersionedFiles ) : | ||
| gutenbergUnversionedFiles; |
There was a problem hiding this comment.
| const isExplicitGutenbergClean = | |
| grunt.cli.tasks.length === 1 && | |
| grunt.cli.tasks[ 0 ] !== 'clean'; | |
| return isExplicitGutenbergClean ? | |
| gutenbergUnversionedFiles.concat( gutenbergVersionedFiles ) : | |
| gutenbergUnversionedFiles; | |
| if( grunt.cli.tasks.length === 1 && grunt.cli.tasks[ 0 ] !== 'clean' ) { | |
| return gutenbergUnversionedFiles.concat( gutenbergVersionedFiles ); | |
| } else { | |
| return gutenbergUnversionedFiles; | |
| } |
Nit: The back to back multiple lines single line functions is a bit hard to grok.
| /** | ||
| * Copy all assets for blocks from Gutenberg to Core. | ||
| * Handles scripts, styles, PHP, and JSON for all block types in a unified way. | ||
| * Generate a list of stable blocks. |
There was a problem hiding this comment.
| * Generate a list of stable blocks. | |
| * Generate a list of stable blocks - meaning blocks that are not marked as experimental. |
When a file was removed from the Gutenberg repository upstream, it currently persists in
src/wp-includes/indefinitely unless someone knows to manually delete it. While the build process copies new files in, it never performs a proper clean.This PR makes a few changes to address this and fix some loosely related pre-existing bugs along the way
First, all Gutenberg-sourced files are now built into
src. Because files were previously built into theWORKING_DIRECTORY, when someone ran a combination ofbuild:devandbuildafter pulling recent changes that modified file names or added/removed files, this sometimes resulted in built files being out of sync, or stale version controlled files insrcdespite them appearing "correct" inbuild.Second, the
clean:gutenbergis now the first task run as part ofbuild:gutenberg. It has also been updated to clean all versioned and unversioned files managed by the Gutenberg-related build scripts. However, only versioned files will only be cleaned ifclean:gutenbergis called directly. Ifgrunt cleanis called, then only unversioned files are cleaned. This is to avoid a dirty state after runninggrunt cleanwhere versioned files have been deleted and not regenerated.Lastly, the
jsFileshas been adjusted so thatgutenbergVersionedFiles,gutenbergUnversionedFilesandwebpackFilesbecome reponsible for the files generated in the respective tasks.Trac ticket: Core-65418.
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 4.7 & 4.8
Used for: Analyzing and documenting the various tasks and scripts and the outcomes of different build paths before creating a first draft of this PR that was adjusted and reviewed by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.